[Perf] Faster block tree (de)serialization - #3349
Conversation
Signed-off-by: ljedrz <ljedrz@users.noreply.github.com>
Signed-off-by: ljedrz <ljedrz@users.noreply.github.com>
Antonio95
left a comment
Antonio95
left a comment
There was a problem hiding this comment.
Great stuff 👌 I think the reason the issue hadn't been caught earlier is that the changes which caused it were merged into staging only recently (#3249).
I have left only minor comment regarding the consistency check in from_state. Happy to approve once we discuss that one.
| } | ||
| ensure!(root_hash == root, "The Merkle tree state has an invalid root"); | ||
|
|
||
| Ok(Self { |
There was a problem hiding this comment.
From what I gather, this initialisation function checks consistency between the "internal root" tree[0] (the root of the largest subtree including all non-empty leaves) and the full tree's root (obtained by hashing the previous value with the empty hash a number of times). But it does not check consistency between tree (the digests of the implicit leaves) and the internal root.
Is my understanding correct? And if so, is there any particular reason for that half-check design? It seems it would make more sense to either check consistency in both cases, or in neither - but I might be missing something.
There was a problem hiding this comment.
this is the same logic as in the existing prepare_append (L#343), just double-checked against the state's root for internal consistency, since we only ever use it against our own cached tree (i.e. we trust it); strictly speaking, we could skip it, but it's relatively cheap
There was a problem hiding this comment.
I see, so it's a cheap sanity check 👌 And doing the whole digests-to-tree[0] check is not justified in terms of time vs. necessity. Approving now.
3 participants
Found while working on ProvableHQ/snarkOS#4361: restarting the nodes took way longer than I anticipated, and it boils down to a flat ~48s (on my machine, ~125s in the CI) cost of loading the block tree cache, which - while still a big improvement for very large ledgers - is a slowdown for small ones.
This PR removes this flat (de)serialization "tax", by introducing a
MerkleTreeStateobject, which is a subset of theMerkleTree, sans its hashers, which can be recreated without having to deserialize them from the cache file.Below is a comparison of the block tree load times between the current state and this PR:
Why haven't we noticed this sooner? Variable window size was added to BHP on 30.04.2026 (dd09be7, efa368b), along with setup and size metrics, which increased the size of the serialized block tree from ~210KiB to 20.7MiB, so what originally cost ~0.5s to deserialize now costs ~48s. Note: those changes were most likely expected, and have their own benefits, with this fallout just being an expected side effect.